fix(sc4): scope npm lock resolution to owning projects - #940

[SkillSpector Review]

Value and readiness

Ready. Per-project ChainMaps replace the global npm name/version map, preserving workspace hoisting while preventing sibling or independent descendants from borrowing unrelated lock pins.

Material findings

No required changes found.

PIC tradeoffs

Independent lockfile audits remain intact even when shrinkwrap takes precedence for manifest resolution. Unsupported alias inference remains unresolved as documented; explicit pins retain precedence. Shared installation maps avoid per-workspace table copies.

Verification and gaps

Read the full diff, #937 and pinned extraction/application/node context. Traced modern and legacy workspace roots, nearest-install precedence, own malformed-lock ownership, shrinkwrap precedence, traversal rejection, aliases, exact pins and budget exits. Reviewed every added regression and both adjusted mapping assertions; the changed paths preserve independent lock vulnerability extraction. Hosted checks: changes SUCCESS, lint SUCCESS, test-unit SUCCESS, OpenCode TypeScript Tests SUCCESS, DCO Check SUCCESS, docker-smoke SUCCESS. This is a read-only static review; no contributor code, tests, imports, builds or scanner invocations were executed.

Reviewed head: 47dea3fa1adf22a807aa4a224a8e8047b742c20a.

[SkillSpector Review]

Value and readiness

Ready for maintainer review. npm manifests now resolve ranges within their owning installation tree, preserving workspace hoisting without borrowing unrelated sibling or independent descendant pins. The prior marked approval had no required changes; this full re-review found none at the new head.

Material findings

No required changes found. static_patterns_supply_chain.py:2199-2310 reserves project-owned lock directories before resolution, gives shrinkwrap precedence, rejects traversal-shaped workspace locations, and builds nearest-install ChainMaps. The call at line 4355 supplies only the manifest directory's map. Explicit manifest pins and independent lockfile audits remain intact; malformed/deep own locks cannot cause ancestor borrowing. Regressions cover v1/v2/v3 locks, component ordering, modern/legacy workspaces, nested installs, uppercase basenames, malformed own locks, shrinkwrap, aliases, bounds and source/parse-error accounting.

PIC tradeoffs

Unsupported alias inference remains conservative as documented. Independent lockfile auditing continues even when another lock controls manifest resolution; shared installation maps avoid workspace table copies.

Verification and gaps

Read the complete four-file diff, exact-head analyzer source, all marked review history and comments/threads, and regression assertions. Exact-head CI run 41516460409: changes, lint, OpenCode TypeScript Tests, DCO and docker-smoke succeeded. test-unit reports 17,833 passed and 15 failed; all 15 failed test IDs exactly match main e14c450b run 41513800480, comprising PowerShell/quote-ownership cases. This establishes no additional failing test ID, while the suite remains red and GitHub reports MERGEABLE/UNSTABLE. No required-check names are configured. Static review only; no contributor code, tests, imports, builds, setup or scanner invocations were executed.

Reviewed head: 675ed530cf2cba91440a9ad0ebb49abc8e25d065.

Source review of ea5aeef → 26d3b7e, followed by completed Greptile review of the same comparison. One remaining alias-shadowing attribution issue is described inline. It also affects the baseline, so this is a gap in the scoped resolution fix rather than a newly introduced regression.

This review used source tracing only. I did not run candidate code, tests, builds, scanner invocations, source/wheel comparisons, the full sample corpus, or live-provider checks; those remain pending. Author-reported validation and the vendor verdict are not substitutes for those checks.

The alias case now preserves the nearest lookup boundary. Local-file links still need the same protection so a workspace dependency cannot inherit an unrelated ancestor registry version.

[SkillSpector Review]

Value and readiness

This PR scopes npm manifest range resolution to the lockfile installation tree that owns each project. Workspace hoisting and nearest-install precedence still work, and sibling or independent descendant projects no longer borrow unrelated pins. I re-reviewed it since 675ed530. Commits 1-4 are unchanged in the range-diff, and the only new commit is 284a6f8d. It fixes the alias-shadowing point from yashrajp22's earlier thread. This round found no new required changes. yashrajp22's open thread on link: true slots still needs to be resolved before merge.

Material findings

No required changes found. At static_patterns_supply_chain.py:2258-2269, an alias slot whose recorded name differs from its slot name is now stored as None in its owner layer. ChainMap.get (built at 2314-2316) stops at that layer, and _apply_locked_versions (2038-2050) leaves the workspace alias unresolved instead of taking the root's registry pin. Root and sibling projects still resolve through their own layers. Explicit manifest pins still take precedence, and _extract_packages_from_npm_lock still audits the alias target on its own. The new tests cover lock v2/v3, package-lock and shrinkwrap, scoped and unscoped names, and both component orders, which is the reported case.

PIC tradeoffs

A workspace alias manifest now stays unresolved. It reports uncertainty instead of resolving the alias target, and the PR description explicitly scopes out full alias-target resolution. The alias target's installed version is still reported through the independent lockfile audit. Independent lockfile auditing continues even when shrinkwrap or another lock controls manifest resolution.

Verification and gaps

I read the full four-file diff against current main, the range-diff since the last reviewed head, and every changed function at this head with its callers and callees. That covers the npm lock collector, alias and name handling, the v1 file: link branch, owner depth precedence, shrinkwrap precedence, _apply_locked_versions, the npm lockfile entry extractor, the pinned-version helper, the package.json extractor, the SC4 OSV and fallback paths, and the call site that selects each manifest directory's map. I traced nearest-install resolution against Node's module lookup for v1/v2/v3 locks, modern and legacy workspaces, nested and vendored node_modules projects, own empty or malformed locks, shrinkwrap plus package-lock, scoped names, aliases, and traversal-shaped locations. Each layout either matched the runtime lookup or stayed unresolved. In this file, current main changes only unrelated SC2/SC3 code, and a trial merge is clean.

Hosted CI at this head: all six checks succeeded (changes, lint, test-unit, OpenCode TypeScript Tests, DCO Check, docker-smoke).

This is a static review: no contributor code, tests, imports, builds or scanner invocations were executed. Only stdlib pathlib/ChainMap behaviour was checked.

Reviewed head: 284a6f8da82bff405190bddbb4a5258bfe478c0a.